[SPARK-5021] [MLlib] Gaussian Mixture now supports Sparse Input #4459

MechCoder · 2015-02-08T20:27:32Z

Following discussion in the Jira.

MechCoder · 2015-02-08T20:30:41Z

ping @tgaloppo and @jkbradley (whenever you are back!)

SparkQA · 2015-02-08T20:32:52Z

Test build #27049 has started for PR 4459 at commit 6d8d4bb.

This patch merges cleanly.

tgaloppo · 2015-02-08T21:16:21Z

mllib/src/main/scala/org/apache/spark/mllib/clustering/GaussianMixture.scala


 import org.apache.spark.annotation.Experimental
-import org.apache.spark.mllib.linalg.{BLAS, DenseMatrix, DenseVector, Matrices, Vector, Vectors}
+import org.apache.spark.mllib.linalg.{Matrices, Vector, Vectors, DenseVector,


alphabetize these

SparkQA · 2015-02-08T21:48:06Z

Test build #27049 has finished for PR 4459 at commit 6d8d4bb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-08T21:48:10Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27049/
Test PASSed.

MechCoder · 2015-02-09T04:38:16Z

@tgaloppo Thanks. I shall fix them in a while. However, does the general code look good to you?

SparkQA · 2015-02-09T06:32:52Z

Test build #27087 has started for PR 4459 at commit c235bd0.

This patch merges cleanly.

SparkQA · 2015-02-09T07:49:59Z

Test build #27087 has finished for PR 4459 at commit c235bd0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-09T07:50:03Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27087/
Test PASSed.

tgaloppo · 2015-02-09T13:21:04Z

mllib/src/main/scala/org/apache/spark/mllib/linalg/BLAS.scala

+  def syr(alpha: Double, x: SparseVector, A: DenseMatrix) {
+    val mA = A.numRows
+    val nA = A.numCols
+    require(mA == nA, s"A is not a square matrix. A: $mA x $nA")


Hmm. Symmetry is a requirement of the syr() function; what is being performed is an incomplete check. The old message mismatched the check, but matched the requirement; perhaps this could be something like "A is not square, and thus cannot be symmetric" ... or something along that line?

MechCoder · 2015-02-09T13:30:14Z

@tgaloppo Thanks for your valuable feedback. Do you have anything more to add as of now?

tgaloppo · 2015-02-09T13:43:53Z

@MechCoder Nothing else stands out to me... I will give it another look after your next commit.

SparkQA · 2015-02-09T17:27:55Z

Test build #27107 has started for PR 4459 at commit 8580799.

This patch merges cleanly.

MechCoder · 2015-02-09T17:29:21Z

@tgaloppo I fixed it up. Can you have a look?

SparkQA · 2015-02-09T17:32:58Z

Test build #27108 has started for PR 4459 at commit 3154f6f.

This patch merges cleanly.

MechCoder · 2015-02-09T17:45:33Z

Also a noob question, but what is the significance of the negative variance in the tests?

SparkQA · 2015-02-09T18:42:48Z

Test build #27107 has finished for PR 4459 at commit 8580799.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-09T18:42:51Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27107/
Test FAILed.

SparkQA · 2015-02-09T18:47:11Z

Test build #27108 has finished for PR 4459 at commit 3154f6f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-09T18:47:15Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27108/
Test PASSed.

tgaloppo · 2015-02-09T19:04:43Z

@MechCoder Do you mean the negative values in the covariance (sigma) matrices? Negative covariance indicates, roughly speaking, that variables move in opposite directions... as one increases, the other decreases (positive covariance indicate they move together, and zero covariance indicate they move independently). The covariance matrix has the variance of the individual vector components on the diagonal, and the covariance between vector components off the diagonal (and is symmetric). Does this help??

tgaloppo · 2015-02-09T19:16:27Z

mllib/src/test/scala/org/apache/spark/mllib/clustering/GaussianMixtureSuite.scala

+      val gmm = new GaussianMixture().setK(1).setSeed(seed).run(data)
+      assert(gmm.weights(0) ~== Ew absTol 1E-5)
+      assert(gmm.gaussians(0).mu ~== Emu absTol 1E-5)
+    }


Should check covariance matrix here. The Esigma given here does not look right. If you need help computing it, let me know.

My mistake; I am sorry.

tgaloppo · 2015-02-09T19:18:46Z

@MechCoder Getting close; just need to finish up the sparse single cluster test.

SparkQA · 2015-02-10T04:22:28Z

Test build #27171 has started for PR 4459 at commit 0679440.

This patch merges cleanly.

AmplabJenkins · 2015-02-10T05:37:07Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27171/
Test PASSed.

MechCoder · 2015-02-10T06:08:58Z

@tgaloppo Alright, thanks for the explanation. Can I know, what makes you think that the covariance matrix is wrong? I calculated it manually and it seems to be right. I added the test and it also passes.

SparkQA · 2015-02-10T06:12:56Z

Test build #27179 has started for PR 4459 at commit e579041.

This patch merges cleanly.

SparkQA · 2015-02-10T07:27:59Z

Test build #27179 has finished for PR 4459 at commit e579041.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-10T07:28:03Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27179/
Test PASSed.

tgaloppo · 2015-02-10T09:10:38Z

LGTM

cc: @jkbradley @mengxr

MechCoder · 2015-02-10T09:21:12Z

@mengxr Just to make it easier for you, a small description. GaussianMixture used to support sparse input, by converting it to DenseVectors, which is non-optimal, in case of a large number of zeros. This PR makes sure that the model works without this conversion happening.

mengxr · 2015-02-10T18:33:16Z

mllib/src/main/scala/org/apache/spark/mllib/linalg/BLAS.scala

@@ -235,12 +235,23 @@ private[spark] object BLAS extends Serializable with Logging {
   * @param x the vector x that contains the n elements.
   * @param A the symmetric matrix A. Size of n x n.
   */
-  def syr(alpha: Double, x: DenseVector, A: DenseMatrix) {
+
+  // Wrapper around the dense and sparse methods.


This is not part of the JavaDoc. We can either move it to the JavaDoc or to the method closure.

MechCoder · 2015-02-10T20:27:47Z

@mengxr Fixed up your comments. Let me know if there is anything else.

SparkQA · 2015-02-10T20:32:31Z

Test build #27230 has started for PR 4459 at commit b997aa3.

This patch merges cleanly.

SparkQA · 2015-02-10T20:42:56Z

Test build #27231 has started for PR 4459 at commit 1b18dab.

This patch merges cleanly.

SparkQA · 2015-02-10T21:46:21Z

Test build #27230 has finished for PR 4459 at commit b997aa3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-10T21:46:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27230/
Test PASSed.

SparkQA · 2015-02-10T21:59:02Z

Test build #27231 has finished for PR 4459 at commit 1b18dab.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-10T21:59:06Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27231/
Test PASSed.

Following discussion in the Jira. Author: MechCoder <[email protected]> Closes #4459 from MechCoder/sparse_gmm and squashes the following commits: 1b18dab [MechCoder] Rewrite syr for sparse matrices e579041 [MechCoder] Add test for covariance matrix 5cb370b [MechCoder] Separate tests for sparse data 5e096bd [MechCoder] Alphabetize and correct error message e180f4c [MechCoder] [SPARK-5021] Gaussian Mixture now supports Sparse Input (cherry picked from commit fd2c032) Signed-off-by: Xiangrui Meng <[email protected]>

mengxr · 2015-02-10T22:06:29Z

LGTM. Merged into master and branch-1.3. Thanks!

MechCoder · 2015-02-11T04:06:00Z

Thanks @tgaloppo and @mengxr . Any idea what to touch in GaussianMixture next? The parallelized Gaussian initialization?

tgaloppo reviewed Feb 8, 2015
View reviewed changes

MechCoder changed the title ~~[SPARK-5021] Gaussian Mixture now supports Sparse Input~~ [SPARK-5021] [MLlib] Gaussian Mixture now supports Sparse Input Feb 9, 2015

tgaloppo reviewed Feb 9, 2015
View reviewed changes

MechCoder force-pushed the sparse_gmm branch from 8580799 to 3154f6f Compare February 9, 2015 17:28

tgaloppo reviewed Feb 9, 2015
View reviewed changes

[SPARK-5021] Gaussian Mixture now supports Sparse Input

e180f4c

MechCoder force-pushed the sparse_gmm branch from 0679440 to 5cb370b Compare February 10, 2015 06:09

Add test for covariance matrix

e579041

mengxr reviewed Feb 10, 2015
View reviewed changes

Rewrite syr for sparse matrices

1b18dab

MechCoder force-pushed the sparse_gmm branch from b997aa3 to 1b18dab Compare February 10, 2015 20:40

asfgit closed this in fd2c032 Feb 10, 2015

MechCoder deleted the sparse_gmm branch February 11, 2015 04:04

[SPARK-5021] [MLlib] Gaussian Mixture now supports Sparse Input #4459

[SPARK-5021] [MLlib] Gaussian Mixture now supports Sparse Input #4459

Conversation

MechCoder commented Feb 8, 2015

MechCoder commented Feb 8, 2015

SparkQA commented Feb 8, 2015

tgaloppo Feb 8, 2015

Choose a reason for hiding this comment

SparkQA commented Feb 8, 2015

AmplabJenkins commented Feb 8, 2015

MechCoder commented Feb 9, 2015

SparkQA commented Feb 9, 2015

SparkQA commented Feb 9, 2015

AmplabJenkins commented Feb 9, 2015

tgaloppo Feb 9, 2015

Choose a reason for hiding this comment

MechCoder commented Feb 9, 2015

tgaloppo commented Feb 9, 2015

SparkQA commented Feb 9, 2015

MechCoder commented Feb 9, 2015

SparkQA commented Feb 9, 2015

MechCoder commented Feb 9, 2015

SparkQA commented Feb 9, 2015

AmplabJenkins commented Feb 9, 2015

SparkQA commented Feb 9, 2015

AmplabJenkins commented Feb 9, 2015

tgaloppo commented Feb 9, 2015

tgaloppo Feb 9, 2015

Choose a reason for hiding this comment

tgaloppo Feb 10, 2015

Choose a reason for hiding this comment

tgaloppo commented Feb 9, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

MechCoder commented Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

tgaloppo commented Feb 10, 2015

MechCoder commented Feb 10, 2015

mengxr Feb 10, 2015

Choose a reason for hiding this comment

MechCoder commented Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

mengxr commented Feb 10, 2015

MechCoder commented Feb 11, 2015